Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: store events to retry #247

Merged
merged 53 commits into from
Feb 4, 2025
Merged

feat: store events to retry #247

merged 53 commits into from
Feb 4, 2025

Conversation

tipogi
Copy link
Collaborator

@tipogi tipogi commented Dec 10, 2024 •
edited
Loading

Pre-submission Checklist

For tests to work you need a working neo4j and redis instance with the example dataset in docker/db-graph

  • Testing: Implement and pass new tests for the new features/fixes, cargo test.
  • Performance: Ensure new code has relevant performance benchmarks, cargo bench

@tipogi tipogi self-assigned this Dec 10, 2024
@amirRamirfatahi
Copy link
Collaborator

2 initial comments to think about:

  1. Can we use Redis as the messaging queue?
  2. being able to identify ghost(?) users.

@SHAcollision SHAcollision changed the title feat: Watcher event fail component feat: retry manager Jan 1, 2025
@tipogi tipogi mentioned this pull request Jan 7, 2025
Merged
2 tasks
SHAcollision and others added 3 commits January 7, 2025 19:38
@SHAcollision
deps: bump axum to 0.8.1 (#276)
b7832c5
@dependabot
deps: bump serde_json from 1.0.134 to 1.0.135 (#281)
2bd53cf 
Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.134 to 1.0.135.
- [Release notes](https://github.com/serde-rs/json/releases)
- [Commits](serde-rs/json@v1.0.134...v1.0.135)

---
updated-dependencies:
- dependency-name: serde_json
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@tipogi
merge main
1239256
@tipogi tipogi marked this pull request as ready for review January 20, 2025 17:06
amirRamirfatahi
amirRamirfatahi reviewed Jan 29, 2025
View reviewed changes
src/events/retry/manager.rs Outdated

/// Initializes a new `RetryManager` with a message-passing channel
impl RetryManager {
pub fn init_channels() -> (ReceiverChannel, SenderChannel) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for refactoring the previous version, but this is still not the best approach. It's causing unnecessary cloning.
Let's refactor to this:

pub struct RetryManager {
    sender: Sender<RetryQueueMessage>
}
pub fn init() -> RetryManager {}
pub async fn process_messages() {}

And then use it like this:

let retry_manager = RetryManager::init();
let x = X::new(retry_manager.sender.clone());
retry_manager.start();

amirRamirfatahi
amirRamirfatahi reviewed Feb 3, 2025
View reviewed changes
Copy link
Collaborator

@amirRamirfatahi amirRamirfatahi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some minor changes needed. Still some older comments are unanswered.
Apart from this, I thought this PR is going to handle retrying the events too? Did we just remove the older retry logic in favor of just storing + no retrying? why? If we were only gonna store them, then why all the channel complexity? We have a receiver that only is going to X::put_to_index()? Couldn't we do this in the main thread that's already processing the event?

src/events/processor.rs Outdated
/// - `event`: The event to be processed
async fn handle_event(&mut self, event: Event) -> Result<(), DynError> {
let mut attempts = 0;
let mut delay = Duration::from_millis(MESSAGE_SEND_RETRY);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for constant names like these I think it's better to include what it is in the name SEND_RETRY_MESSAGE_DELAY

src/events/mod.rs
.into())
}
};
} // drop the pubky_client lock
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does pubky_client have a lock? isn't it Arc?

src/events/mod.rs
let blob = response.bytes().await?;
let resource = self.parsed_uri.resource;

// Use the new importer from pubky-app-specs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason for this comment? At what point does the importer stop being new? :))

@tipogi tipogi merged commit 2a72419 into main Feb 4, 2025
3 checks passed
@tipogi tipogi deleted the feat/retry-manager branch February 4, 2025 14:52
@SHAcollision SHAcollision changed the title feat: retry manager feat: store events to retry Feb 4, 2025
amirRamirfatahi added a commit that referenced this pull request Feb 7, 2025
@amirRamirfatahi
Squashed commit of the following:
d1d59a4 
commit b533fac
Merge: e9ffcc2 c265041
Author: amir <[email protected]>
Date:   Thu Feb 6 23:08:59 2025 -0500

    Merge branch 'main' into feature/migrations

commit c265041
Author: tipogi <[email protected]>
Date:   Thu Feb 6 17:01:40 2025 +0100

    chore: improve poll events error handling (#323)

commit 6282b69
Author: SHAcollision <[email protected]>
Date:   Wed Feb 5 11:47:03 2025 -0400

    Fix: readme unresolved merge conflict

commit e9ffcc2
Author: amir <[email protected]>
Date:   Tue Feb 4 19:45:03 2025 -0500

    Fix merge flaw

commit bf8f6cc
Merge: 4afd26d 50acf66
Author: amir <[email protected]>
Date:   Tue Feb 4 19:42:02 2025 -0500

    Merge branch 'main' into feature/migrations

commit 50acf66
Author: amirRamirfatahi <[email protected]>
Date:   Tue Feb 4 10:44:05 2025 -0500

    Merge pull request #249 from pubky/refactor/standalone-tests

    tests: refactor service tests as standalone

commit 2a72419
Author: tipogi <[email protected]>
Date:   Tue Feb 4 15:52:07 2025 +0100

    feat: retry manager (#247)

commit 8c29f5b
Author: tipogi <[email protected]>
Date:   Tue Feb 4 15:24:00 2025 +0100

    chore: refactor hot tags endpoint (#294)

    * refactor hot tags endpoint

    * wrap all the hot tags indexes under Cache:Tags:Post:Hot:xxx. Refactor stream.rs file

    * improve docstring in tag/stream

    * Delete old indexes: Sorted:Tags:Global:Hot:[tag_name] and Tag:Taggers:[tag_name]

    * Fix taggers endpoint. Now retrieves from the right indexes the taggers data

    * Fix query for taggers reach and add integration tests

    * service tests successful

    * fmt linter fixed

    * adjust the position of the prefix argument in Redis-related function signatures

    * add integration test for the hot tag endpoint, covering reach and timeframe

    * lint fixes

    * requested changes

    * missing argument

    * fixed taggers integration test

    * small fixes

    * fixed 1st day month timeframe based tests

commit 4bd3465
Author: SHAcollision <[email protected]>
Date:   Mon Feb 3 12:45:39 2025 -0400

    chore(tests): no fail fast on CI testing

commit f061df9
Author: SHAcollision <[email protected]>
Date:   Mon Feb 3 09:03:25 2025 -0400

    Feat: switch to nextest (#312)

commit 0aed790
Author: tipogi <[email protected]>
Date:   Mon Feb 3 11:21:24 2025 +0100

    fix: empty tags param bug on post stream endpoint (#309)

    * fix post stream endpoint

    * cover test

commit 1d48330
Author: tipogi <[email protected]>
Date:   Mon Feb 3 11:21:04 2025 +0100

    update nexus wireframe (#310)

commit f1df386
Author: SHAcollision <[email protected]>
Date:   Fri Jan 31 09:46:38 2025 -0400

    Feat: use pubky id from specs (#307)

    * Feat: use pubky id from specs

    * chore: pin main specs

commit 1407c21
Author: amirRamirfatahi <[email protected]>
Date:   Wed Jan 22 18:43:32 2025 -0500

    Fix: user search duplicate bug (#260)

    * Fix user search

    * fix file urls serde

commit 4afd26d
Author: amir <[email protected]>
Date:   Wed Jan 15 03:23:43 2025 -0500

    fix import in example

commit 5104f45
Author: amir <[email protected]>
Date:   Wed Jan 15 03:06:28 2025 -0500

    fix import

commit d5dbbc6
Merge: 447040e de24c37
Author: amir <[email protected]>
Date:   Wed Jan 15 03:03:27 2025 -0500

    Merge branch 'main' into feature/migrations

commit 447040e
Author: amir <[email protected]>
Date:   Wed Jan 15 02:34:41 2025 -0500

    Move to_snake_case to utils

commit ef37e15
Author: amir <[email protected]>
Date:   Sat Jan 4 05:41:05 2025 -0500

    fix lint

commit a22638d
Author: amir <[email protected]>
Date:   Sat Jan 4 05:20:33 2025 -0500

    perform cleanups

commit 918f3a9
Author: amir <[email protected]>
Date:   Thu Dec 19 04:26:21 2024 -0500

    fix lint

commit 2041bba
Author: amir <[email protected]>
Date:   Tue Dec 17 02:27:18 2024 -0500

    Add new migration command and add readme and examples
amirRamirfatahi added a commit that referenced this pull request Feb 27, 2025
@amirRamirfatahi
Squashed commit of the following:
dbfa50b 
commit fcf5656
Merge: 0d47f78 a74eb2e
Author: Amirreza Amirfatahi <[email protected]>
Date:   Tue Feb 25 03:24:40 2025 -0500

    Merge branch 'main' into feature/files-processing

commit 0d47f78
Author: tipogi <[email protected]>
Date:   Tue Feb 25 08:30:26 2025 +0100

    add better error handling related to retry queue (#338)

    Co-authored-by: Amirreza Amirfatahi <[email protected]>

commit 4d5d7c7
Merge: 6b7139d 104dd69
Author: Amirreza Amirfatahi <[email protected]>
Date:   Mon Feb 24 18:04:34 2025 -0500

    Merge branch 'main' into feature/files-processing

commit 6b7139d
Merge: 3acb8e2 0f3e8f6
Author: Amirreza Amirfatahi <[email protected]>
Date:   Thu Feb 20 20:45:50 2025 -0500

    Merge branch 'main' into feature/files-processing

commit 3acb8e2
Merge: 1aebfd2 45f6523
Author: Amirreza Amirfatahi <[email protected]>
Date:   Tue Feb 18 07:21:03 2025 -0500

    Merge branch 'main' into feature/files-processing

commit 1aebfd2
Author: Amirreza Amirfatahi <[email protected]>
Date:   Thu Feb 13 02:12:28 2025 -0500

    refactor serve dir

commit 2c21de1
Author: Amirreza Amirfatahi <[email protected]>
Date:   Thu Feb 13 01:35:38 2025 -0500

    move avatar route to statics

commit b140403
Author: Amirreza Amirfatahi <[email protected]>
Date:   Thu Feb 13 01:25:47 2025 -0500

    refactor routes and add swagger docs for static routes

commit 7d24a01
Author: Amirreza Amirfatahi <[email protected]>
Date:   Tue Feb 11 07:02:33 2025 -0500

    fix lint

commit 59f1045
Author: Amirreza Amirfatahi <[email protected]>
Date:   Tue Feb 11 05:31:05 2025 -0500

    refactor avatar serving

commit f0a1f18
Author: SHAcollision <[email protected]>
Date:   Mon Feb 10 20:34:57 2025 -0400

    Fix delete test

commit 89c1de3
Author: SHAcollision <[email protected]>
Date:   Mon Feb 10 20:28:42 2025 -0400

    Fix avatar endpoint

commit 7094694
Merge: f25acb6 a5b7781
Author: Amirreza Amirfatahi <[email protected]>
Date:   Mon Feb 10 23:29:53 2025 -0500

    Merge branch 'main' into feature/files-processing

commit f25acb6
Author: amir <[email protected]>
Date:   Sat Feb 8 00:15:06 2025 -0500

    fix tests

commit 50007e7
Merge: cddeb31 101db09
Author: amir <[email protected]>
Date:   Fri Feb 7 22:13:01 2025 -0500

    Merge branch 'main' into feature/files-processing

commit cddeb31
Author: amir <[email protected]>
Date:   Fri Feb 7 18:51:31 2025 -0500

    Set webp as conversion target for image processor

commit 5e42617
Author: amir <[email protected]>
Date:   Fri Feb 7 01:25:52 2025 -0500

    fix tests

commit d1d59a4
Author: amir <[email protected]>
Date:   Fri Feb 7 01:19:50 2025 -0500

    Squashed commit of the following:

    commit b533fac
    Merge: e9ffcc2 c265041
    Author: amir <[email protected]>
    Date:   Thu Feb 6 23:08:59 2025 -0500

        Merge branch 'main' into feature/migrations

    commit c265041
    Author: tipogi <[email protected]>
    Date:   Thu Feb 6 17:01:40 2025 +0100

        chore: improve poll events error handling (#323)

    commit 6282b69
    Author: SHAcollision <[email protected]>
    Date:   Wed Feb 5 11:47:03 2025 -0400

        Fix: readme unresolved merge conflict

    commit e9ffcc2
    Author: amir <[email protected]>
    Date:   Tue Feb 4 19:45:03 2025 -0500

        Fix merge flaw

    commit bf8f6cc
    Merge: 4afd26d 50acf66
    Author: amir <[email protected]>
    Date:   Tue Feb 4 19:42:02 2025 -0500

        Merge branch 'main' into feature/migrations

    commit 50acf66
    Author: amirRamirfatahi <[email protected]>
    Date:   Tue Feb 4 10:44:05 2025 -0500

        Merge pull request #249 from pubky/refactor/standalone-tests

        tests: refactor service tests as standalone

    commit 2a72419
    Author: tipogi <[email protected]>
    Date:   Tue Feb 4 15:52:07 2025 +0100

        feat: retry manager (#247)

    commit 8c29f5b
    Author: tipogi <[email protected]>
    Date:   Tue Feb 4 15:24:00 2025 +0100

        chore: refactor hot tags endpoint (#294)

        * refactor hot tags endpoint

        * wrap all the hot tags indexes under Cache:Tags:Post:Hot:xxx. Refactor stream.rs file

        * improve docstring in tag/stream

        * Delete old indexes: Sorted:Tags:Global:Hot:[tag_name] and Tag:Taggers:[tag_name]

        * Fix taggers endpoint. Now retrieves from the right indexes the taggers data

        * Fix query for taggers reach and add integration tests

        * service tests successful

        * fmt linter fixed

        * adjust the position of the prefix argument in Redis-related function signatures

        * add integration test for the hot tag endpoint, covering reach and timeframe

        * lint fixes

        * requested changes

        * missing argument

        * fixed taggers integration test

        * small fixes

        * fixed 1st day month timeframe based tests

    commit 4bd3465
    Author: SHAcollision <[email protected]>
    Date:   Mon Feb 3 12:45:39 2025 -0400

        chore(tests): no fail fast on CI testing

    commit f061df9
    Author: SHAcollision <[email protected]>
    Date:   Mon Feb 3 09:03:25 2025 -0400

        Feat: switch to nextest (#312)

    commit 0aed790
    Author: tipogi <[email protected]>
    Date:   Mon Feb 3 11:21:24 2025 +0100

        fix: empty tags param bug on post stream endpoint (#309)

        * fix post stream endpoint

        * cover test

    commit 1d48330
    Author: tipogi <[email protected]>
    Date:   Mon Feb 3 11:21:04 2025 +0100

        update nexus wireframe (#310)

    commit f1df386
    Author: SHAcollision <[email protected]>
    Date:   Fri Jan 31 09:46:38 2025 -0400

        Feat: use pubky id from specs (#307)

        * Feat: use pubky id from specs

        * chore: pin main specs

    commit 1407c21
    Author: amirRamirfatahi <[email protected]>
    Date:   Wed Jan 22 18:43:32 2025 -0500

        Fix: user search duplicate bug (#260)

        * Fix user search

        * fix file urls serde

    commit 4afd26d
    Author: amir <[email protected]>
    Date:   Wed Jan 15 03:23:43 2025 -0500

        fix import in example

    commit 5104f45
    Author: amir <[email protected]>
    Date:   Wed Jan 15 03:06:28 2025 -0500

        fix import

    commit d5dbbc6
    Merge: 447040e de24c37
    Author: amir <[email protected]>
    Date:   Wed Jan 15 03:03:27 2025 -0500

        Merge branch 'main' into feature/migrations

    commit 447040e
    Author: amir <[email protected]>
    Date:   Wed Jan 15 02:34:41 2025 -0500

        Move to_snake_case to utils

    commit ef37e15
    Author: amir <[email protected]>
    Date:   Sat Jan 4 05:41:05 2025 -0500

        fix lint

    commit a22638d
    Author: amir <[email protected]>
    Date:   Sat Jan 4 05:20:33 2025 -0500

        perform cleanups

    commit 918f3a9
    Author: amir <[email protected]>
    Date:   Thu Dec 19 04:26:21 2024 -0500

        fix lint

    commit 2041bba
    Author: amir <[email protected]>
    Date:   Tue Dec 17 02:27:18 2024 -0500

        Add new migration command and add readme and examples

commit 6c0b01e
Author: amir <[email protected]>
Date:   Mon Jan 27 19:05:55 2025 -0500

    Refactor and clean up

commit 46e85ca
Author: Amirreza Amirfatahi <[email protected]>
Date:   Wed Jan 22 17:06:26 2025 -0500

    Add imagemagick installation to build pipeline

commit 3aec0d4
Merge: 00a86da 372a9a6
Author: Amirreza Amirfatahi <[email protected]>
Date:   Wed Jan 22 16:47:27 2025 -0500

    Merge branch 'main' into feature/files-processing

commit 00a86da
Author: Amirreza Amirfatahi <[email protected]>
Date:   Wed Jan 22 16:36:36 2025 -0500

    fix content type bug

commit d100914
Author: Amirreza Amirfatahi <[email protected]>
Date:   Wed Jan 22 16:24:14 2025 -0500

    Refactor static routes

commit c542f15
Author: amir <[email protected]>
Date:   Thu Jan 16 01:05:03 2025 -0500

    fix build and lint

commit b4efff5
Merge: 10713bb 85a9bf6
Author: amir <[email protected]>
Date:   Wed Jan 15 21:45:23 2025 -0500

    Merge branch 'main' into feature/files-processing

commit 10713bb
Author: amir <[email protected]>
Date:   Mon Jan 13 08:05:25 2025 -0500

    replace status code import from reqwest to axum

commit 9c4812d
Author: amir <[email protected]>
Date:   Thu Jan 9 08:20:48 2025 -0500

    Refactor and cleanup static routes

commit 928bfca
Author: amir <[email protected]>
Date:   Fri Jan 3 11:47:51 2025 -0500

    resolve more refactoring comments

commit b0b71df
Author: amir <[email protected]>
Date:   Fri Jan 3 11:12:37 2025 -0500

    refactor files processing

commit e613ee3
Merge: 077afaa ac21ac5
Author: amir <[email protected]>
Date:   Fri Dec 13 17:19:58 2024 -0500

    Merge branch 'main' into feature/files-processing

commit 077afaa
Author: amir <[email protected]>
Date:   Wed Dec 11 05:31:35 2024 -0500

    revert breaking changes of lint

commit 99db66b
Author: amir <[email protected]>
Date:   Wed Dec 11 00:31:45 2024 -0500

    fix lint issues

commit 02402d1
Author: amir <[email protected]>
Date:   Wed Dec 11 00:20:59 2024 -0500

    fix tests tokio runtime

commit 8ce61f9
Merge: 923dc1f 287caa1
Author: amir <[email protected]>
Date:   Wed Dec 11 00:18:43 2024 -0500

    Merge branch 'main' into feature/files-processing

commit 923dc1f
Author: amir <[email protected]>
Date:   Tue Dec 10 23:15:34 2024 -0500

    Add image mock, tests for image and improve code quality

commit 41b2871
Author: amir <[email protected]>
Date:   Tue Dec 10 03:23:53 2024 -0500

    Add more tests and fix file urls

commit b21b2cc
Author: amir <[email protected]>
Date:   Mon Dec 9 20:05:14 2024 -0500

    fix tests and bugs

commit 9502a36
Author: amir <[email protected]>
Date:   Fri Sep 27 00:23:16 2024 -0400

    add image and video processing

commit 6de7abc
Author: amir <[email protected]>
Date:   Tue Sep 24 10:57:05 2024 -0400

    Change static files path into a folder per file

commit 6847a2d
Author: amir <[email protected]>
Date:   Thu Sep 19 16:34:26 2024 -0400

    Add file router to main routes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amirRamirfatahi amirRamirfatahi amirRamirfatahi left review comments

@SHAcollision SHAcollision SHAcollision approved these changes

Assignees

@tipogi tipogi

Labels
enhancement New feature or request 🔮 nexus 👀 watcher
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tipogi @amirRamirfatahi @SHAcollision